Extend backdating checks to imports and globals #59735
Open
+259
−83
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, backdated binding warnings were only emitted for constants. This commit extends the backdating mechanism to handle imports and globals. This change was requested by triage to improve compatibility of the new binding partition mechanism given issues like #58511. With this change the behavior is as follows: ```julia julia> function foo() include_string(Main, "a = 42") return a end julia> foo() WARNING: Detected access to binding `Main.a` in a world prior to its definition world. Julia 1.12 has introduced more strict world age semantics for global bindings. !!! This code may malfunction under Revise. !!! This code will error in future versions of Julia. Hint: Add an appropriate `invokelatest` around the access to this binding. To make this warning an error, and hence obtain a stack trace, use `julia --depwarn=error`. 42 ``` With `julia --depwarn=error`, the access is rejected: ```julia julia> foo() ERROR: UndefVarError: `a` not defined in `Main` ``` I am still not a huge fan of the backdating mechanism in general, but I think this is the least objectionable of the alternatives and provides a big warning to let people know that something is wrong.
Keno
changed the title
oscardssmith
added
the
needs pkgeval
|
I pretty much think this is too risky to put in this late in the release process, and that the current status quo is the lesser evil. For example, this PR fails CI on a random assortment of tests, and Revise stops working: |
vtjnash
added a commit
that referenced
this pull request
Oct 6, 2025
Also remove `isdefined` shim, as that feels less logical to me with the current implementation that uses invokelatest. If the implementation was changed to use `invokenext`, I think it would make sense to add that back and also to update `names` to return these also. But invoke next (similar to backdated constants and #59735) prints an incorrect suggestion of using invokelatest to get the same behavior. That message is correct with this current commit state, but wrong relative to the current implementations that use backdated.
vtjnash
added a commit
that referenced
this pull request
Oct 6, 2025
|
Rebinding test failure is #59613, so we should merge that first. |
|
effects.jl is the combination of a pre-existing codegen model bug (will PR separately) with #59613 |
|
vtjnash
reviewed
Oct 7, 2025
Comment on lines
+689
to
+690
| # Test that implicit imports are NOT backdated | ||
| module BackdateNoImplicitTest |
There was a problem hiding this comment.
Please tell Claude its implementation of this particular test is rubbish and to redo it correctly. Approximately:
diff --git a/test/worlds.jl b/test/worlds.jl
index 6dcef169fba..b69f92d2d01 100644
--- a/test/worlds.jl
+++ b/test/worlds.jl
-# Test that implicit imports are NOT backdated
+# Test that conflicting imports are still detected and handled
module BackdateNoImplicitTest
using Test
module Source1
@@ -693,7 +689,22 @@ module BackdateNoImplicitTest
shared_name() = 2
end
const world_before = Base.get_world_counter()
- using .Source1, .Source2
- @test_throws UndefVarError shared_name()
+ @test_throws UndefVarError shared_name
+ using .Source1
+ @test shared_name === Source1.shared_name
+ @test shared_name() === 1
+ if Base.JLOptions().depwarn <= 1
+ @test Source1.shared_name === @test_warn "Detected access to binding `BackdateNoImplicitTest.shared_name`" Base.invoke_in_world(world_before, getglobal, @__MODULE__, :shared_name)
+ else
+ @test_throws UndefVarError Base.invoke_in_world(world_before, getglobal, @__MODULE__, :shared_name)
+ end
+ @test isdefinedglobal(@__MODULE__, :shared_name)
+ @test !Base.invoke_in_world(world_before, isdefinedglobal, @__MODULE__, :shared_name)
+
+ using.Source2
+ @test_throws UndefVarError shared_name
+ @test !isdefinedglobal(@__MODULE__, :shared_name)
+ @test_throws UndefVarError Base.invoke_in_world(world_before, getglobal, @__MODULE__, :shared_name)
+ @test !isdefinedglobal(@__MODULE__, :shared_name)
@test !Base.invoke_in_world(world_before, isdefinedglobal, @__MODULE__, :shared_name)
end
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, backdated binding warnings were only emitted for constants. This commit extends the backdating mechanism to handle imports and globals. This change was requested by triage to improve compatibility of the new binding partition mechanism given issues like #58511.
With this change the behavior is as follows:
With
julia --depwarn=error, the access is rejected:I am still not a huge fan of the backdating mechanism in general, but I think this is the least objectionable of the alternatives and provides a big warning to let people know that something is wrong.
Largely written by Claude.